fix(kotlin): stop deprecations pointing at deprecated replacements - #731
fix(kotlin): stop deprecations pointing at deprecated replacements#731ayaangazali wants to merge 1 commit into
Conversation
|
Heads up on the red Both are red on main too, at I opened #736 with the evidence and a one-line guard. Nothing to do on this PR; every other check here is green. |
511f1cd to
2736edc
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughUpdated Kotlin deprecation messages for model, STT, TTS, and VAD APIs. The messages now reference current replacement APIs and stream lifecycle methods. ChangesKotlin API guidance
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: ⚪ Minimal · up to This change only corrects migration guidance for deprecated Kotlin APIs without changing signatures, runtime behavior, or replacement mechanics. No actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
2736edc to
9d75f38
Compare
|
Rebased onto Still reproduces. Re-ran the check against current Following any of those trades one deprecation warning for another. This retargets them at the live replacements. For context on scope: the same defect exists in Swift and Web and is #762, and |
9d75f38 to
327dfe9
Compare
|
Still reproduces on So the six extension deprecations this PR touches send a caller to a symbol that immediately warns again and carries its own The replacements they point at all exist and are not deprecated ( Text-only change to |
…ated API stopSynthesis/stopSpeaking in both SDKs tell the caller to use tts.stop(), which is itself deprecated in favour of the SpeechHandle returned by speak(). Following the advice trades one deprecation warning for another, and on Swift `renamed:` gives Xcode a fix-it that performs the bad migration for you. Retarget all four at the SpeechHandle, matching the wording tts.stop() already uses. Swift drops `renamed:` for `message:` because the replacement is a returned handle, not a symbol that can be substituted at the call site. Kotlin's equivalents were fixed in RunanywhereAI#731.
Six @deprecated messages on the legacy extension surface name a namespace method that is itself deprecated, so following the migration advice lands the caller on a second deprecation: unloadModel -> models.unload(category) @deprecated -> unloadAll(category) stopSynthesis -> tts.stop() @deprecated -> SpeechHandle stopSpeaking -> tts.stop() @deprecated -> SpeechHandle streamVAD -> vad.detectStream(audio, options) @deprecated -> openStream(format, options) resetVAD -> vad.detectStream @deprecated -> openStream(format, options) transcribeStream -> stt.transcribeStream(audio, ...) @deprecated -> openStream(format, options) Each now names the replacement the namespace's own annotation points at, so one hop reaches a supported API: models.unloadAll(category) interrupt() on the SpeechHandle returned by tts.speak() vad.openStream(format, options) / close() on the VadStream it returns stt.openStream(format, options) Messages only. No signatures, bodies, or annotations targets change.
327dfe9 to
4710704
Compare
What is wrong
Six
@Deprecatedmessages on the legacy extension surface tell the caller to move to a namespace method that is itself deprecated, so following the advice lands them on a second deprecation:unloadModelmodels.unload(category)@Deprecated("Use unloadAll(category).")stopSynthesistts.stop()@Deprecated("Use the SpeechHandle returned by speak().")stopSpeakingtts.stop()streamVADvad.detectStream(audio, options)@Deprecated("Use vad.openStream(format, options).")resetVADvad.detectStreamtranscribeStreamstt.transcribeStream(audio, options)@Deprecated("Use stt.openStream(format, options).")The IDE shows the strikethrough and the message, the caller migrates, and gets struck through again.
ReplaceWithis not involved in any of these, so nothing auto-applies, but the written guidance is a dead end and no gate covers it.check_deprecated_surfaces.shtracks which deprecated surfaces exist against an allowlist; it does not check that a deprecation's target is supported.What this changes
Each message now names the replacement the namespace's own annotation points at, so one hop reaches a supported API:
models.unloadAll(category)interrupt()on theSpeechHandlereturned bytts.speak()vad.openStream(format, options), andclose()on theVadStreamit returns for the reset casestt.openStream(format, options)I took each target from the deprecated namespace method's own annotation rather than picking one, and confirmed the members exist:
ModelsNamespace.unloadAll(:266),TtsNamespace.speak(:60) returningSpeechHandlewithinterrupt()(Results.kt:252),SttNamespace.openStream(:189), andVadStream.close()(Results.kt:315).Message strings only. No signature, body, or
ReplaceWithtarget changes, so this cannot alter behaviour.How I found them
Collected the deprecated methods declared inside
public/api/*Namespace.kt, then looked for any@Deprecatedmessage elsewhere naming<namespace>.<method>for that set. Six hits, four files. After the change the same scan reports zero.Worth noting the scan produced 31 raw hits at first and 25 were false positives, because a deprecated extension and the live namespace method often share a simple name (
generate,rerank,diarize). Those messages are correct and I left them alone; only the six above name a target that really is deprecated.Verification
./gradlew :compileDebugKotlin --rerun-tasks -x :buildLocalJniLibs -x :downloadJniLibs -x :syncAndroidRuntimeLibs -Prunanywhere.useLocalNatives=false: BUILD SUCCESSFUL./gradlew ktlintMainSourceSetCheck: cleangit statusshows only the four source files; the codegen the Gradle build runs left nothing behindNo test added: these are annotation strings, and a test asserting the text of a deprecation message would pin prose rather than behaviour.
Summary by CodeRabbit